Skip to content

docs: comprehensive docstring updates for JointLimiterInterface - #3412

Open
Rudyy75 wants to merge 11 commits into
ros-controls:masterfrom
Rudyy75:fix/jointlimiter-init-docstring
Open

docs: comprehensive docstring updates for JointLimiterInterface#3412
Rudyy75 wants to merge 11 commits into
ros-controls:masterfrom
Rudyy75:fix/jointlimiter-init-docstring

Conversation

@Rudyy75

@Rudyy75 Rudyy75 commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Initially addresses #3410 by updating the @brief for the init overload that accepts soft_joint_limits.

Per maintainer feedback, expanded the scope to unify, standardize, and complete the Doxygen docstrings (@param, @return) across the rest of the JointLimiterInterface APIs.

Fixes #3410
Related to #3391

Is this user-facing behavior change?

Did you use Generative AI?

Yes, Claude Sonnet 4.6

Additional Information

TODOs

To send us a pull request, please:

  • Fork the repository.
  • Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
  • Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • Commit to your fork using clear commit messages.
  • Send a pull request, answering any default questions in the pull request interface.
  • Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Jun 24, 2026
saikishor
saikishor requested changes Jun 24, 2026
View reviewed changes
Comment thread joint_limits/include/joint_limits/joint_limiter_interface.hpp

/**
* Wrapper init method that accepts the joint names and their limits directly
* @brief Initialize joints from directly provided names, limits, and soft joint limits.

@saikishor saikishor Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a detailed overview of the parameters and also for the other parts of the codebase

@Rudyy75 Rudyy75 Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the docstring with @param entries for all parameters and checked the rest of the codebase for similar issues.

@Rudyy75 Rudyy75 mentioned this pull request Jun 24, 2026
Closed
@Rudyy75
Rudyy75 requested a review from saikishor June 25, 2026 17:11
@mergify

mergify Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This pull request is in conflict. Could you fix it @Rudyy75?

Copilot AI reviewed Jul 4, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Doxygen documentation for joint_limits::JointLimiterInterface init/configure APIs to better reflect the actual parameters and return values, addressing the mismatch called out in issue #3410.

Changes:

  • Add missing @return documentation for the primary init(...) overload.
  • Update the init(...) overload docstring to explicitly include soft_joint_limits and document its parameters/return.
  • Expand Doxygen docs for init(...) node/lifecycle-node overloads and add brief docs for configure(...) and reset_internals().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread joint_limits/include/joint_limits/joint_limiter_interface.hpp
@christophfroehlich
Rephrase docstring
0a158d4 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@christophfroehlich christophfroehlich moved this from WIP to Needs review in Review triage Jul 4, 2026
@codecov

codecov Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.56%. Comparing base (0b09bf4) to head (c8a6217).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #3412   +/-   ##
=======================================
  Coverage   89.56%   89.56%           
=======================================
  Files         168      168           
  Lines       21419    21419           
  Branches     1662     1662           
=======================================
  Hits        19184    19184           
  Misses       1533     1533           
  Partials      702      702
Flag Coverage Δ
unittests 89.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/include/joint_limits/joint_limiter_interface.hpp 51.51% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Rudyy75
Rudyy75 requested a review from Copilot July 9, 2026 08:46
Copilot AI reviewed Jul 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@Rudyy75

Rudyy75 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

I have resolved the merge conflicts and updated all the docstrings with @param and @return descriptions as requested. Let me know if anything else is needed.

@Rudyy75

Rudyy75 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Hey @saikishor, I have resolved all the threads and updated the docstrings as requested. Christoph has also approved. Could you take a look and approve?

@Rudyy75
Rudyy75 requested a review from Copilot July 20, 2026 16:27
Copilot AI reviewed Jul 20, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread joint_limits/include/joint_limits/joint_limiter_interface.hpp
@Rudyy75 Rudyy75 changed the title fix: update docstring for JointLimiterInterface::init overload to include soft_joint_limits docs: comprehensive docstring updates for JointLimiterInterface Jul 20, 2026
@Rudyy75

Rudyy75 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Hey @saikishor, I've updated the PR title and description to match the expanded documentation scope you requested. Ready for your final look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@christophfroehlich christophfroehlich christophfroehlich approved these changes
@erickisos erickisos Awaiting requested review from erickisos
@homalozoa homalozoa Awaiting requested review from homalozoa
@Juliaj Juliaj Awaiting requested review from Juliaj
@saikishor saikishor Awaiting requested review from saikishor

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

Review triage
Status: Needs review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Inaccurate init method description in JointLimiterInterface.

4 participants

@Rudyy75 @christophfroehlich @saikishor